Reduce CppInterOp Emscripten shared library size #655

mcbarton · 2025-07-01T14:42:23Z

Description

Please include a summary of changes, motivation and context for this PR.

@vgvassilev This change locally reduced the Emscripten shared library size to 47/48 Mb (depends of whether you use dir -lh or du -lh). It still passed all tests and xeus-cpp still could run all the notebook cells it could before (exception throwing is broken in CppInterOps deployment at the moment for some reason). I have cleared the cache of all llvm 20 Emscripten builds for this PR.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

Bug fix
New feature
Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

I have read the contribution guide recently

codecov · 2025-07-01T14:51:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.18%. Comparing base (98e07e7) to head (ba6ed4c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #655   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files           9        9           
  Lines        3853     3853           
=======================================
  Hits         3051     3051           
  Misses        802      802

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

anutosh491 · 2025-07-01T14:57:00Z

It's a compile time optimization flag I think.

Do we need to force it like this everywhere or can we have cmake handle this ?

mcbarton · 2025-07-01T15:24:36Z

It's a compile time optimization flag I think.

Do we need to force it like this everywhere or can we have cmake handle this ?

It is both a link and compile time optimisation. See https://emscripten.org/docs/tools_reference/emcc.html#emcc-oz .

anutosh491 · 2025-07-01T15:26:14Z

Yeah but we still don't want users to look into/pass flags correct during the build correct?

We should be able to have cmake handle this.

mcbarton · 2025-07-01T15:32:16Z

Yeah but we still don't want users to look into/pass flags correct during the build correct?

We should be able to have cmake handle this.

I really don't understand the point your trying to make. Using EMCC_CFLAGS in this way to perfectly legitimate way of doing the Emscripten build. Emscripten is designed to be effected by modifying this flag (see https://emscripten.org/docs/tools_reference/emcc.html#environment-variables). If a user changes what we have in the documentation for this flag, and messes something up, then that is on them. I don't understand how putting it in a cmake option changes that.

anutosh491 · 2025-07-01T15:39:46Z

My point is simple. We already have compile time and link time flags being passed through cmake, I don't see a need to pass this externally.

vgvassilev · 2025-07-01T16:27:45Z

I think what @anutosh491 means is that we need to make this flag as part of our default compiler flags in the CMakeLists.txt.

mcbarton · 2025-07-01T18:31:46Z

@vgvassilev @anutosh491 I first tried locally using -DCMAKE_CXX_FLAGS="-Dwait4=__syscall_wait4 -Oz" -DCMAKE_C_FLAGS="-Oz" -DCMAKE_STATIC_LINKER_FLAGS="-Oz" (since I thought this might be the closest thing to EMCC_CFLAGS) and the llvm build crashed 15% into the build. I then tried -DCMAKE_CXX_FLAGS="-Dwait4=__syscall_wait4 -Oz" -DCMAKE_C_FLAGS="-Oz" which although I could build llvm with this option, had no effect on the size of CppInterOps shared library, or the llvm libraries. I do not know how to proceed now. If you have any other cmake options you would like me to try, let me know. If you don't, I think we should proceed using EMCC_CFLAGS.

mcbarton · 2025-07-01T21:57:03Z

Putting a note here in case I forget. Using EMCC_CFLAGS="-flto" in the llvm build doesn't end up effecting the shared library size on its own. Used in conjuncation with Oz then the size falls slightly to 45/46 Mb.

mcbarton · 2025-07-02T10:43:46Z

I have updated this PR to use the flto flag as well, and to compile+link CppInterOp shared library with these flags too. This reduces the shared library size to 44/45 Mb.

mcbarton · 2025-07-07T19:47:07Z

@vgvassilev I have now changed this PR to a cmake only solution, instead of using EMCC_CFLAGS. Hopefully that means this PR is ready to go in once the ci passes. Can you review and approve if your happy? The Emscripten cache will need deleting before merging this PR, so that the deployment gets the new smaller library.

mcbarton · 2025-07-07T21:46:18Z

The failing job is because I accidentally deleted the wrong cache part way through the ci run. Will rerun tomorrow to show the failed job passing. Noticed that with this change, the Emscripten caches are twice the size they were before, so will need to delete Emscripten llvm 19 jobs for this to go in I think.

anutosh491 · 2025-07-08T07:47:02Z

.github/workflows/emscripten.yml

@@ -194,6 +194,9 @@ jobs:
                        -DLLVM_ENABLE_LIBPFM=OFF                        \
                        -DCLANG_BUILD_TOOLS=OFF                         \
                        -DLLVM_NATIVE_TOOL_DIR=$NATIVE_DIR 		\
+                        -DCMAKE_C_FLAGS_RELEASE="-Oz -g0" \
+                        -DCMAKE_CXX_FLAGS_RELEASE="-Oz -g0" \
+                        -DLLVM_ENABLE_LTO=Full \


I am not sure, we necessarily need to change the way llvm is built for this.

We absolutely need to change the llvm build to get this reduction in size of Emscripten shared library. The majority of the size reduction this PR provides comes from adding these flags to the llvm build.

anutosh491 · 2025-07-08T07:50:08Z

lib/CppInterOp/CMakeLists.txt

  target_link_options(clangCppInterOp
    PRIVATE "SHELL: -s WASM_BIGINT"
    PRIVATE "SHELL: -s SIDE_MODULE=1"
    PRIVATE "SHELL: ${SYMBOLS_LIST}"
+    PRIVATE "SHELL: -Oz"
+    PRIVATE "SHELL: -flto"


Hmm why is lto being involved here ? I've tried using it in the past and I remember it behaves notoriously.
How is it helping us here ?

Adding these flags gets us a further reduction in the size of the Emscripten shared library of a few Mb. Needed them for both compile and link flags for it to have any effect. I didn't see any bad side effects your comment suggests (I don't know exactly what you mean by behaves notoriously).

mcbarton requested a review from vgvassilev July 1, 2025 14:42

mcbarton force-pushed the Reduce-Emscripten-shared-library-size branch from ee75d01 to d9f6630 Compare July 1, 2025 15:25

mcbarton force-pushed the Reduce-Emscripten-shared-library-size branch from db280e0 to de7101c Compare July 2, 2025 10:38

mcbarton changed the title ~~Use EMCC_CFLAGS="-Oz" during Emscripten llvm build to reduce CppInterOp shared library size to 48Mb~~ Reduce CppInterOp Emscripten shared library size to 48Mb Jul 2, 2025

mcbarton changed the title ~~Reduce CppInterOp Emscripten shared library size to 48Mb~~ Reduce CppInterOp Emscripten shared library size Jul 2, 2025

mcbarton force-pushed the Reduce-Emscripten-shared-library-size branch from 6869da8 to effdaf8 Compare July 7, 2025 19:44

Use Oz and flto flags to reduce Emscripten shared library size

ba6ed4c

mcbarton force-pushed the Reduce-Emscripten-shared-library-size branch from effdaf8 to ba6ed4c Compare July 7, 2025 19:55

anutosh491 self-requested a review July 8, 2025 07:42

anutosh491 reviewed Jul 8, 2025

View reviewed changes

Reduce CppInterOp Emscripten shared library size #655

Are you sure you want to change the base?

Reduce CppInterOp Emscripten shared library size #655

Uh oh!

Conversation

mcbarton commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Testing

Checklist

Uh oh!

codecov bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

anutosh491 commented Jul 1, 2025

Uh oh!

mcbarton commented Jul 1, 2025

Uh oh!

anutosh491 commented Jul 1, 2025

Uh oh!

mcbarton commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anutosh491 commented Jul 1, 2025

Uh oh!

vgvassilev commented Jul 1, 2025

Uh oh!

mcbarton commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcbarton commented Jul 1, 2025

Uh oh!

mcbarton commented Jul 2, 2025

Uh oh!

mcbarton commented Jul 7, 2025

Uh oh!

mcbarton commented Jul 7, 2025

Uh oh!

anutosh491 Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

mcbarton Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

anutosh491 Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

mcbarton Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcbarton commented Jul 1, 2025 •

edited

Loading

codecov bot commented Jul 1, 2025 •

edited

Loading

mcbarton commented Jul 1, 2025 •

edited

Loading

mcbarton commented Jul 1, 2025 •

edited

Loading